fix: relax strict min-backbone-shift-types from 5 to 4#6
Draft
fix: relax strict min-backbone-shift-types from 5 to 4#6
Conversation
- Remove duplicate entries (*.txt was listed twice) - Organize into logical sections with comments - Add exception for trizod/potenci/data/ directory - Use proper gitignore patterns (directories with trailing /) - POTENCI data files are now included as required dependencies
- Add 6 CSV tables extracted from inline strings - Add comprehensive README documenting data format - Data sourced from Nielsen & Mulder (2018) POTENCI algorithm Files added: - tablecent.csv: Central residue chemical shifts - tablenei.csv: Neighbor residue corrections - tabletermcorrs.csv: Terminal corrections - tabletempk.csv: Temperature coefficients - tablecombdevs.csv: Combinatorial deviations - tablephshifts.csv: pH-dependent shifts - README.md: Comprehensive documentation
- Add comprehensive type hints (ShiftDict, CorrectionDict, etc.) - Replace unsafe eval() calls with safe float conversion - Implement CSV-based data loading with caching - Add PhysicalConstants dataclass - Remove all backward compatibility wrappers - Update module docstring with academic references Security: Eliminates eval() vulnerability Performance: Cached data loading with lru_cache Maintainability: Type-safe, well-documented API
- Add comprehensive module docstring - Fix typo: logging.waring() to logging.warning() - Update outdated comments (python2.x to python3.10+) - Replace ##-style comments with proper documentation - Update to use new constants API (PHYSICAL_CONSTANTS, load_* functions) - Improve CLI documentation in main()
- Export modern API: load_central_shifts, PHYSICAL_CONSTANTS, etc. - Remove legacy exports: R, a, b, cutoff, e, ncycles - Add module docstring - Update __all__ for clean public API
- Remove setup.py (replaced by pyproject.toml) - Add uv.lock for reproducible dependencies - Configure hatchling to include potenci/data files - Update build system to use modern Python packaging standards Migration: setup.py → pyproject.toml + uv Build backend: hatchling Lock file: uv.lock for reproducibility
- Run ruff check --fix --unsafe-fixes on all modules - Apply ruff format for consistent code style - Fix import ordering, comparison operators, nested if statements - Remove unused variables and imports - Add explicit exception handling (no bare excepts) - Rename functions to follow snake_case convention: - get_pH → get_ph - convChi2CDF → conv_chi2_cdf - get_offset_corrected_wSCS → get_offset_corrected_wscs - Rename exceptions to follow Error suffix convention: - Found → FoundError - OffsetTooLargeException → OffsetTooLargeError - OffsetCausedFilterException → OffsetCausedFilterError - FilterException → FilterError - Fix lambda loop variable binding issue - Add exception chaining with "from e"
…equires-python - Replace deprecated np.float with np.float64 (removed in NumPy 1.24+) - Replace eval() with float() in read_csv_pkaoutput (security fix) - Fix argument count mismatch in getpredshifts_arr -> getphcorrs_arr call - Remove dead code: unused log_fun(), no-op str(i+1) statements - Bump requires-python from >=3.8 to >=3.9 (BooleanOptionalAction, dict |) - Exclude test/ from ruff config (pre-existing issues, not part of package) - Add implementation plan document for BMRB expert suggestions Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace blanket *.csv/*.txt/*.zip patterns with specific directory rules for clarity and transparency. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Modern NMR experiments commonly yield HN, N, CO, CA (4 types) without HA. CB is technically side-chain. Requiring 5 excludes many valid datasets. Also add clarifying comment on ionic-strength units (Molar). Addresses expert suggestion #9. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
tsenoner
added a commit
that referenced
this pull request
Mar 5, 2026
Values 1-14 K are physically impossible for liquid-state protein NMR (below helium liquefaction). These are almost certainly Celsius values (e.g. 5°C = 278 K, 10°C = 283 K) that were recorded without units. Addresses expert suggestion #6. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
min-backbone-shift-typesfrom 5 to 4Context
Expert suggestion #9: requiring 5 backbone shift types excludes many valid datasets where HA is not assigned.
🤖 Generated with Claude Code